fix(tree): use url.PathUnescape for path parameters#4674
Open
baltasarblanco wants to merge 3 commits into
Open
fix(tree): use url.PathUnescape for path parameters#4674baltasarblanco wants to merge 3 commits into
baltasarblanco wants to merge 3 commits into
Conversation
Replace url.QueryUnescape with url.PathUnescape when decoding wildcard parameter values in the radix tree. Per RFC 3986, URL path segments preserve '+' as a literal character; the '+' to space convention belongs to application/x-www-form-urlencoded, not to URL paths. The change affects only configurations using UseEscapedPath or UseRawPath together with UnescapePathValues=true. The default routing path is unchanged because URL.Path is already decoded by net/http upstream of getValue. Two existing test cases in TestUnescapeParameters previously asserted the incorrect '+' to space behavior and have been updated. A new test case verifies that '%2B' (percent-encoded '+') still decodes to '+'. Fixes gin-gonic#3850
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4674 +/- ##
==========================================
- Coverage 99.21% 98.38% -0.83%
==========================================
Files 42 48 +6
Lines 3182 3160 -22
==========================================
- Hits 3157 3109 -48
- Misses 17 42 +25
- Partials 8 9 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Gin’s radix tree parameter decoding to use url.PathUnescape (instead of url.QueryUnescape) when UnescapePathValues=true, aligning path parameter semantics with RFC 3986 by preserving literal + characters in URL path segments.
Changes:
- Replace
url.QueryUnescapewithurl.PathUnescapewhen unescaping:paramand*catchAllparameter values in the router tree. - Update
TestUnescapeParametersexpectations so literal+remains+, and add coverage to confirm%2Bstill decodes to+.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tree.go | Switch path-parameter unescaping from query-style to path-style unescaping to preserve + literals. |
| tree_test.go | Adjust and extend tests to match the corrected + handling and ensure %2B decoding remains correct. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Checklist
Please ensure your pull request meets the following requirements:
masterbranch.docs/doc.md.Description
Replace
url.QueryUnescapewithurl.PathUnescapewhen decoding wildcard parameter values in the radix tree. Per RFC 3986, URL path segments preserve+as a literal character; the+→ space convention belongs toapplication/x-www-form-urlencoded, not to URL paths. This aligns Gin with the upstreamhttprouterand Go stdlib semantics.Scope of impact
Affects only configurations using
UseEscapedPathorUseRawPathtogether withUnescapePathValues=true. The default routing path is unchanged becauseURL.Pathis already decoded bynet/httpupstream ofgetValue, sounescapeisfalseand neither function is invoked.Behavior change (called out explicitly)
A URL path param containing a literal
+(e.g./items/a+b) previously decoded toa b; it now correctly decodes toa+b. Percent-encoded+(%2B) still decodes to+. Clients needing a space in a path segment should URL-encode it as%20(RFC 3986).Tests
TestUnescapeParameterspreviously asserted the incorrect+→ space behavior and have been updated.%2Bstill decodes to+.-race,-tags=nomsgpack,-tags=go_json,-tags=sonic(with--ldflags="-checklinkname=0").Fixes #3850